-
Notifications
You must be signed in to change notification settings - Fork 0
setup error handling including demo #88
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
barluq
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for try to implement solution into the NPM, however we should to have our NPM free from any framework dependencies (just React and TS). NextJS directived should not to be included.
add-use-client.mjs
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is pure Next syntax and should not to be used in general FE NPM.
barluq
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please avoid to push any NextJS magic to NPM. It should be pure JS/TS, nothing more. If there is a NextJS funtionality reqired, it should be handled inside NextJS per application.
package.json
Outdated
| "@deck.gl/react": "^9.2.2", | ||
| "@gisatcz/deckgl-geolib": "1.12.0-dev.5", | ||
| "@gisatcz/ptr-be-core": "^0.0.1-dev.9", | ||
| "@gisatcz/ptr-be-core": "^0.0.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "@gisatcz/ptr-be-core": "^0.0.2", | |
| "@gisatcz/ptr-be-core": "^0.0.7", |
rollup.config.js
Outdated
| output: [ | ||
| {file: pkg.main, format: 'cjs', sourcemap: true}, // CommonJS output | ||
| {file: pkg.module, format: 'esm', sourcemap: true} // ES module output | ||
| {file: pkg.main, format: 'cjs', sourcemap: true, banner: "'use client';"}, // CommonJS output |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No NextJS magic in NPM please. We can handle this part per Next app, not in NPM. Also we dont want to have all mandatory parts use client by default (from layouts etc.)
| @@ -0,0 +1,86 @@ | |||
| 'use client'; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope
barluq
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
No description provided.